KAFKA-20111: Handle pre-4.1 brokers in kafka-configs.sh for groups#21385
KAFKA-20111: Handle pre-4.1 brokers in kafka-configs.sh for groups#21385AndrewJSchofield wants to merge 7 commits intoapache:trunkfrom
Conversation
chia7712
left a comment
There was a problem hiding this comment.
@AndrewJSchofield thanks for this fix
| case GroupType => | ||
| adminClient.listGroups().all.get.asScala.map(_.groupId).toSet ++ | ||
| adminClient.listConfigResources(java.util.Set.of(ConfigResource.Type.GROUP), new ListConfigResourcesOptions).all().get().asScala.map(_.name).toSet | ||
| adminClient.listGroups().all.get.asScala.map(_.groupId).toSet ++ listGroupConfigResources(adminClient).map(resources => resources.asScala.map(_.name).toSet).getOrElse(Set() ++ entityName) |
There was a problem hiding this comment.
entityName is None, so Set() ++ entityName could be replaced by Set.empty
There was a problem hiding this comment.
True. I do speak Scala like it's a foreign language unfortunately.
| } | ||
|
|
||
| private def listGroupConfigResources(adminClient: Admin): Option[java.util.Collection[ConfigResource]] = { | ||
| try { |
There was a problem hiding this comment.
try {
Some(adminClient.listConfigResources(java.util.Set.of(ConfigResource.Type.GROUP), new ListConfigResourcesOptions).all.get)
} catch {
// (KIP-1142) 4.1+ admin client vs older broker: treat UnsupportedVersion as None
case e: ExecutionException if e.getCause.isInstanceOf[UnsupportedVersionException] => None
}There was a problem hiding this comment.
@AndrewJSchofield are you happy with this comment ? 😄
the latest commit d917d47 seems to not handle this comment
There was a problem hiding this comment.
I used slightly different words for the comment text. But, I see. I didn't quite get the second part of this. I'll take a look a bit later today.
| public ListConfigResourcesResult listConfigResources(Set<ConfigResource.Type> configResourceTypes, ListConfigResourcesOptions options) { | ||
| ConfigResource.Type type = configResourceTypes.iterator().next(); | ||
| assertEquals(ConfigResource.Type.GROUP, type); | ||
| future.completeExceptionally(new UnsupportedVersionException("The v0 ListConfigResources only supports CLIENT_METRICS")); |
There was a problem hiding this comment.
Should we add a test case for other exceptions to ensure they are propagated correctly and not swallowed
KIP-1142 introduced
Admin.listConfigResources()for listing resourceswhich have configurations, but which may not actually exist as run-time
entities such as consumer groups which have not yet had any members.
Internally, this works by using v1 of the
LIST_CONFIG_RESOURCESRPCwhich is only supported in AK 4.1 and later. As a result, if you try to
describe the config for groups with older brokers, they do not support
v1 of the RPC and fail with
UnsupportedVersionException. This PRhandles the exception in the config tool since the exception is harmless
and gives the same behaviour as we used to get with earlier versions.